As long as default value for attributes is used, it should look rather like:

<!-- wp:query -->
 <div class="wp-block-query"></div>
 <!-- /wp:query -->

I'll investigate. I'm wondering if it has to do with these properties being objects that is not so common in core blocks. For example queryId is not here 🤔

I wouldn't be surprised. I think you know already my opinion about using complex key/value-based objects as attributes. I would avoid them in the first place, so if that creates an issue, we could use the opportunity that deprecation is inevitable to refactor that as well. Anyway, I hope that is something else and you don't have to end up with a nuclear option 🤣

In some cases (Query being the main case that comes in my mind) it makes sense to use objects and we could always explore adding better support for them. Anyway..

How do you feel about this:494aa75

We could always use isEqual but felt like we could avoid the extra call for most cases that is not needed (not type object.

@youknowriad, @gziolo , @mcsf

A similar change has been proposed in a PR at some point and was rejected (don't remember why, maybe performance). It would be good to try to find this PR and the related discussions :)

Is this the one you had in mind: #26162? It's a bit different but has to do with object default values.

I don't think performance is an issue here, since the only core block that has default object props is just the Query block.

This one I think #27348

Isn't it a breaking change? It should be covered with a unit test if we decide to take this approach. What should we do about arrays?

Of course I'll add tests if this will be the way to go. I think we could also handle arrays the same way but first only perform the simple equality value check to avoid the extra call for most cases (performance).

I'm not sure how it's a breaking change. It will maybe affect some third party blocks by removing the default object from its comment delimiters. The default value will still be used in block manipulation in the editor, no?

I'm not entirely sure how this change relates to this PR and whether it's mandatory but I personally don't have big objections here if it's mandatory, it seems the discussions on the linked PR were mostly about performance impact.

Performance was one concern, but the other was the semantic distinction between no value and empty value (i.e. undefined !== {}).

But yeah, in this case I'm not sure how it relates to the current PR — what am I missing?

It all started from the deprecation snapshot here: #30804 (comment).

If we are okay to leave the snapshot as it was with the serialized query object, I can remove this change and move forward this PR with the tests fixing/check.

Thanks for the link, I see it now. I don't think this is an easy question. We'd have to weigh the trade-offs:

• Using isEqual in the serialiser would simplify the case of attributes with deep-valued defaults, which would in a sense "fix" how default values.
• On the other hand, the serialiser has worked like this for four years now (and two in Core), and the fix might cause unexpected changes in third-party blocks who have come to rely on this quirk.
• I feel less strongly today about what I said in December in Fix an issue with the serializer not ignoring attribute values of object attribute type when the value is equal to the default. #27348 (comment), but there is still a point to consider there.
• Finally, obviously, there's the question of performance associated with deep comparison. I don't expect this to be a major factor, in practice, but we would have to test.

I'm in favour of keeping the focus of this PR on the alignment win, and leaving the serialisation matter aside.

On the other hand, the serialiser has worked like this for four years now (and two in Core), and the fix might cause unexpected changes in third-party blocks who have come to rely on this quirk.

That is my main concern with this proposal. Another thing I worry about is that when comparing arrays it's unclear if the order of items should be taken into account. I guess, it's better to leave it unchanged and just open a follow-up issue to document better the current behavior in the documentation.

On the other hand, the serialiser has worked like this for four years now (and two in Core), and the fix might cause unexpected changes in third-party blocks who have come to rely on this quirk.

I guess, it's better to leave it unchanged and just open a follow-up issue to document better the current behavior in the documentation.

I agree. This is a totally different and bigger issue that needs resolving on its own. I'll revert the related change.